Skip to content

fix(clickhouse): strip virtual catalogs from executed queries and inserts - #6036

Open
mday-io wants to merge 6 commits into
SQLMesh:mainfrom
mday-io:mday/fix-clickhouse-insert-virtual-catalog
Open

fix(clickhouse): strip virtual catalogs from executed queries and inserts#6036
mday-io wants to merge 6 commits into
SQLMesh:mainfrom
mday-io:mday/fix-clickhouse-insert-virtual-catalog

Conversation

@mday-io

@mday-io mday-io commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

When a ClickHouse gateway is used alongside catalog-aware gateways, SQLMesh injects a virtual catalog (__<gateway>__ by default, or the configured virtual_catalog) so every model has a 3-part name. ClickHouse doesn't support catalogs, so that prefix has to be stripped before SQL reaches the engine. Some DDL paths already handled this (create_view, alter_table, delete_from's target table, create_schema), but the queries themselves still carried the catalog. As a result, INSERT ... SELECT, CREATE TABLE ... AS SELECT and subqueries inside DELETE ... WHERE all failed.

This PR strips the virtual catalog centrally in ClickhouseEngineAdapter._to_sql, so it covers every rendered statement:

  • Any table or column reference whose catalog equals the injected _default_catalog has its catalog removed.
  • References to other catalogs and string literals are left untouched.
  • The caller's expression is never mutated. It is only copied when a reference actually needs rewriting, so large INSERT ... VALUES statements aren't deep-copied for no reason.
  • Stripping only happens after inject_virtual_catalog() has run. This matches the existing strip sites, so a ClickHouse-only project with virtual_catalog configured behaves exactly as before.

Test Plan

Added to tests/core/engine_adapter/test_clickhouse.py:

  • test_virtual_catalog_stripped_from_execute_queries (parametrized) covers INSERT ... SELECT, a plain SELECT, and a JOIN that mixes the virtual catalog with another catalog plus a string literal containing the catalog name. It also checks that the input expression isn't mutated.
  • test_virtual_catalog_stripped_from_ctas_and_delete checks the exact SQL emitted by ctas() and by delete_from() with a subquery.

Both tests fail without the fix. All 38 tests in test_clickhouse.py pass, and so does ruff/mypy on the changed files.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

@mday-io
fix(clickhouse): strip virtual catalogs from executed queries and ins…
50b4f9a 
…erts

Signed-off-by: mday-io <mdaytn@gmail.com>
@mday-io
fix(clickhouse): strip virtual catalog from all executed statements
74f820e 
Strip the injected virtual catalog from every rendered expression rather
than only queries and inserts, so CTAS and DELETE statements no longer
leak it. Drop the fallback to the configured virtual_catalog when none was
injected, matching the other catalog strip sites, and only copy the
expression when a reference actually needs rewriting.

Consolidate the tests: the unconfigured-catalog test never enabled the
feature, and the fallback test only covered the removed fallback.

Signed-off-by: mday-io <mdaytn@gmail.com>
@mday-io
Merge branch 'main' into mday/fix-clickhouse-insert-virtual-catalog
9ceed6e
@StuffbyYuki
StuffbyYuki self-requested a review October 1, 2026 04:00
@StuffbyYuki

StuffbyYuki commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

@mday-io No blocker from me, but a few things:

  1. Two methods build SQL as plain strings and skip _to_sql. _create_table_like and _rename_table put their SQL together as strings (with an f-string or .sql()) before calling execute(), so the new stripping never sees them. @set_catalog only strips the last table argument, so the first one keeps the virtual catalog:

    create_table_like("__ch_gw__.db.tgt", "__ch_gw__.db.src")
  -> CREATE TABLE __ch_gw__.db.tgt AS db.src
rename_table("__ch_gw__.db.tgt", "__ch_gw__.db.src")
  -> RENAME TABLE "__ch_gw__"."db"."tgt" TO "db"."src"

    This existed before your change and doesn't affect normal model runs, but it can be hit through direct calls such as dbt's adapter.rename_relation. Wrapping both table names in self._strip_virtual_catalog(...) in those two methods would close it. Otherwise, we might want to reword "covers every rendered statement" in the description

  2. Add a test for the no-injection case. All the virtual-catalog tests call inject_virtual_catalog() first. A test showing that a query like SELECT ... FROM __ch_gw__.mydb.source is left unchanged when injection never happens would lock in the promise that ClickHouse-only projects behave the same as before.

mday-io added 3 commits October 2, 2026 21:26
@mday-io
fix(clickhouse): strip virtual catalog from string-built statements
c287055 
`execute()` sends string statements verbatim, so the central stripping in
`_to_sql` never sees them. `_create_table_like`, `_exchange_tables`,
`_rename_table` and the table/column comment builders still sent the
injected virtual catalog to ClickHouse. This broke insert-overwrite
(incremental model loads) and silently dropped comments.

Route those table names through `_strip_virtual_catalog`, and parse string
names with the ClickHouse dialect when stripping. `_create_table_like` now
renders quoted identifiers, consistent with the rename/exchange statements.

Also add a regression test that three-part names are left untouched when
no virtual catalog has been injected.

Signed-off-by: mday-io <mdaytn@gmail.com>
@mday-io
test(clickhouse): drop duplicate plain SELECT virtual catalog case
615feff 
The INSERT ... SELECT case already covers the same table and column
references, and every regression that fails the plain SELECT case also
fails it.

Signed-off-by: mday-io <mdaytn@gmail.com>
@mday-io
Merge branch 'main' into mday/fix-clickhouse-insert-virtual-catalog
f691113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StuffbyYuki StuffbyYuki StuffbyYuki approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mday-io @StuffbyYuki